Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Use proper types for configure methods (Extension/Mark/Node) #5421

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

rfgamaral
Copy link
Contributor

@rfgamaral rfgamaral commented Jul 30, 2024

Changes Overview

Our Typist package (based on Tiptap) is still using v2.3.0 for now, and while attempting a quick upgrade to the latest version, some type checks started to fail (ref).

One of the issues is that calling Extension.create().configure() returns Node<any, any>, whereas before it would return Node<Options, Storage>. This PR fixes that by making sure that when .configure() is called for an Extension, Node, or Mark, the correct type is returned.

Would love it if this could be reviewed, merged, and release soon, so that we can upgrade our Tiptap version.

Testing Done

Built Tiptap locally and checked the generated .d.ts files, the expected types were there.

Verification Steps

Same as above. Build Tiptap, and check that the .d.ts files are generated with the currect types for the .configure() method.

Checklist

  • I have created a changeset for this PR if necessary.
  • My changes do not break the library.
  • I have added tests where applicable.
  • I have followed the project guidelines.
  • I have fixed any lint issues.

Copy link

netlify bot commented Jul 30, 2024

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit c40c62b
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/66a92b82efe09d0008e90b1a
😎 Deploy Preview https://deploy-preview-5421--tiptap-embed.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@nperez0111 nperez0111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Type inference let us down here.

@bdbch bdbch merged commit 068559d into ueberdosis:develop Jul 31, 2024
14 checks passed
@rfgamaral rfgamaral deleted the node-mark-extension-types branch August 1, 2024 08:14
@nperez0111 nperez0111 mentioned this pull request Aug 6, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants